Skip to content

Conversation

@siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Oct 8, 2025

Fixes #19046

Before: https://stackblitz.com/edit/axg8fqmk?file=src%2FDemo.tsx
After: https://stackblitz.com/edit/54tkqjct?file=src%2FDemo.tsx

  • select all then deselect each row, it should work.

The Bug

When using a controlled exclude model with tree data and row selection propagation, the grid corrupts the model by treating excluded row IDs as selected row IDs.

Step-by-Step Bug Flow

1. User provides controlled exclude model

<DataGridPro
  treeData
  rowSelectionModel={{ type: 'exclude', ids: new Set([1]) }}
  rowSelectionPropagation={{ descendants: true, parents: true }}
  onRowSelectionModelChange={handleChange}
/>

Meaning: "All rows selected EXCEPT row 1 (Thomas)"

2. Grid calls getPropagatedRowSelectionModel

When the controlled prop is provided, syncControlledState (line 849) calls:

const newSelectionModel = apiRef.current.getPropagatedRowSelectionModel(propRowSelectionModel);

3. Bug: Function treats excluded IDs as selected IDs

Before Fix (lines 436-466):

const getPropagatedRowSelectionModel = (inputSelectionModel) => {
  // No check for exclude model type! ❌
  if (!isNestedData || !applyAutoSelection || ...) {
    return inputSelectionModel;
  }

  const propagatedSelectionModel = {
    type: inputSelectionModel.type,  // 'exclude'
    ids: new Set(inputSelectionModel.ids),  // Set([1]) - EXCLUDED rows!
  };

  const selectionManager = createRowSelectionManager(propagatedSelectionModel);

  // Iterate over EXCLUDED row IDs ❌
  for (const id of inputSelectionModel.ids) {  // id = 1 (Thomas)
    findRowsToSelect(  // ❌ Wrong function for excluded rows!
      apiRef, tree, id,
      props.rowSelectionPropagation?.descendants,  // true
      props.rowSelectionPropagation?.parents,  // true
      (rowId) => selectionManager.select(rowId),  // ❌ Removes from exclude set!
    );
  }
};

4. What happens inside the propagation

Tree structure:

Thomas (id: 1) - EXCLUDED
  ├─ Robert (id: 2)
  ├─ Karen (id: 3)
  └─ Nancy (id: 4)

Propagation flow:

  1. findRowsToSelect is called with id = 1 (Thomas - an EXCLUDED row)
  2. It finds children: [2, 3, 4]
  3. For each child, it calls: selectionManager.select(childId)

What select() does for exclude models:

// In gridRowSelectionManager.ts
select(id) {
  if (this.model.type === 'exclude') {
    this.model.ids.delete(id);  // ❌ Removes from exclude set!
  }
}

5. The corruption

Starting model:

{ type: 'exclude', ids: Set([1]) }
// Meaning: "All selected except Thomas"

After propagation:

{ type: 'exclude', ids: Set() }
// Meaning: "All rows selected!" ❌

What happened:

  • Row 1 (Thomas) was in the exclude set
  • select() was called on its children (2, 3, 4)
  • Since they weren't in the exclude set, nothing was deleted
  • But the semantic error is: we iterated over excluded rows and called findRowsToSelect on them!

The Root Cause

Semantic mismatch:

  • findRowsToSelect is designed for SELECTION propagation
  • Exclude model IDs represent UNSELECTED rows
  • Calling findRowsToSelect on unselected rows and using select() to "propagate" them corrupts the model

Why exclude models don't need propagation:

When a user actually deselects a row via UI interaction, propagation ALREADY happens correctly:

// In selectRow (lines 294-302)
} else if (applyAutoSelection) {
  findRowsToDeselect(  // ✅ Correct function
    apiRef, tree, id,
    props.rowSelectionPropagation?.descendants,
    props.rowSelectionPropagation?.parents,
    (rowId) => selectionManager.unselect(rowId),  // ✅ Adds to exclude set
  );
}

Result after UI deselection:

{ type: 'exclude', ids: Set([1, 2, 3, 4]) }  // ✅ Already complete!

The model is semantically complete - no "propagation reapplication" needed.

The Solution

Skip propagation for exclude models in getPropagatedRowSelectionModel

Location: useGridRowSelection.ts:440

Change:

const getPropagatedRowSelectionModel = (inputSelectionModel) => {
  if (
    !isNestedData ||
    !applyAutoSelection ||
    inputSelectionModel.type === 'exclude' ||  // ← NEW: Skip for exclude models
    (inputSelectionModel.ids.size === 0 && inputSelectionModel.type === 'include')
  ) {
    return inputSelectionModel;  // Return unchanged
  }

  // Propagation logic below only runs for include models now
  // ...
};

Test

Added a test to ensure that the exclude model is unchanged when getting from getPropagatedRowSelectionModel


@siriwatknp siriwatknp added type: bug It doesn't behave as expected. scope: data grid Changes related to the data grid. feature: Selection Related to the data grid Selection feature labels Oct 8, 2025
@mui-bot
Copy link

mui-bot commented Oct 8, 2025

Deploy preview: https://deploy-preview-19846--material-ui-x.netlify.app/

Bundle size report

Bundle Parsed size Gzip size
@mui/x-data-grid 🔺+20B(+0.01%) 🔺+1B(0.00%)
@mui/x-data-grid-pro 🔺+20B(0.00%) 🔺+2B(0.00%)
@mui/x-data-grid-premium 🔺+20B(0.00%) 🔺+3B(0.00%)
@mui/x-charts 0B(0.00%) 0B(0.00%)
@mui/x-charts-pro 0B(0.00%) 0B(0.00%)
@mui/x-charts-premium 0B(0.00%) 0B(0.00%)
@mui/x-date-pickers 0B(0.00%) 0B(0.00%)
@mui/x-date-pickers-pro 0B(0.00%) 0B(0.00%)
@mui/x-tree-view 0B(0.00%) 0B(0.00%)
@mui/x-tree-view-pro 0B(0.00%) 0B(0.00%)

Details of bundle changes

Generated by 🚫 dangerJS against 440a1be

@siriwatknp siriwatknp marked this pull request as ready for review October 8, 2025 06:37
@siriwatknp siriwatknp requested a review from a team October 8, 2025 06:40
@MBilalShafi MBilalShafi changed the title [Data Grid] Fix tree data unable to deselect row for exclude model [DataGrid] Fix tree data unable to deselect row for exclude model Oct 15, 2025
Copy link
Member

@MBilalShafi MBilalShafi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still have some bleeding edges around the exclude model with nested data (unrelated to this specific issue) like https://stackblitz.com/edit/axg8fqmk-ex2nb3fw
I will try to refactor the row selection logic to adapt it better to both the types.

@siriwatknp
Copy link
Member Author

We still have some bleeding edges around the exclude model with nested data (unrelated to this specific issue) like https://stackblitz.com/edit/axg8fqmk-ex2nb3fw I will try to refactor the row selection logic to adapt it better to both the types.

What's the issue? Could you link to a Github issue?

@siriwatknp siriwatknp merged commit 830cd10 into mui:master Oct 16, 2025
21 checks passed
@MBilalShafi
Copy link
Member

We still have some bleeding edges around the exclude model with nested data (unrelated to this specific issue) like stackblitz.com/edit/axg8fqmk-ex2nb3fw I will try to refactor the row selection logic to adapt it better to both the types.

What's the issue? Could you link to a Github issue?

I think we don't have one yet. I'll recheck and create one.

@michelengelen
Copy link
Member

We still have some bleeding edges around the exclude model with nested data (unrelated to this specific issue) like stackblitz.com/edit/axg8fqmk-ex2nb3fw I will try to refactor the row selection logic to adapt it better to both the types.

What's the issue? Could you link to a Github issue?

I think we don't have one yet. I'll recheck and create one.

@MBilalShafi did you already create an issue for this?

@MBilalShafi
Copy link
Member

We still have some bleeding edges around the exclude model with nested data (unrelated to this specific issue) like stackblitz.com/edit/axg8fqmk-ex2nb3fw I will try to refactor the row selection logic to adapt it better to both the types.

What's the issue? Could you link to a Github issue?

I think we don't have one yet. I'll recheck and create one.

@MBilalShafi did you already create an issue for this?

Not yet. I was testing a few different issues around this problem to add more context to it.
Will create one soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: Selection Related to the data grid Selection feature scope: data grid Changes related to the data grid. type: bug It doesn't behave as expected.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[data grid] Unable to unselect treeData rows after using "Select all" checkbox with controlled row selection

4 participants